Skip to content

fix: collapse loopback variants in normalize_host to prevent tenant split#2619

Open
AIandI0x1 wants to merge 3 commits into
block:mainfrom
AIandI0x1:fix/normalize-host-loopback-collapse
Open

fix: collapse loopback variants in normalize_host to prevent tenant split#2619
AIandI0x1 wants to merge 3 commits into
block:mainfrom
AIandI0x1:fix/normalize-host-loopback-collapse

Conversation

@AIandI0x1

@AIandI0x1 AIandI0x1 commented Jul 23, 2026

Copy link
Copy Markdown

Problem

normalize_host (buzz-core::tenant) and normalize_relay_url (buzz-core::relay) disagree on loopback address handling:

  • normalize_relay_url collapses localhost, 127.0.0.1, and [::1] to 127.0.0.1
  • normalize_host did not collapse loopback variants

This causes a silent tenant split in local development:

  1. The desktop app spawns agents with ws://127.0.0.1:3000 (after normalize_relay_url)
  2. The frontend connects via ws://localhost:3000 (default)
  3. The relay's bind_community uses the raw Host header, so 127.0.0.1:3000 and localhost:3000 map to different communities
  4. Agents discover 0 channels because their community has no channel data
  5. The UI shows no channels because its community is different/empty

Additionally, normalize_url in buzz-auth::nip98 did not collapse loopback variants either, causing NIP-98 HTTP auth 401 failures when the client signs auth events with http://localhost:3000/query but the relay expects http://127.0.0.1:3000/query.

Fix

  1. buzz-core::tenant — Add normalize_loopback_host() helper and call it from normalize_host. Collapses localhost, 127.0.0.1, 0.0.0.0, and [::1] to 127.0.0.1 (port preserved). This mirrors the loopback collapsing already in normalize_relay_url.

  2. buzz-auth::nip98 — Update normalize_url to normalize the host via normalize_host, so NIP-98 URL comparison agrees with community binding.

  3. Tests — Updated affected tests across buzz-core, buzz-auth, and buzz-relay to use the canonical 127.0.0.1 form. The previous test loopback_aliases_are_distinct_hosts (which asserted localhost ≠ 127.0.0.1) is replaced with loopback_aliases_collapse_to_same_host.

Verification

  • cargo test -p buzz-core --lib tenant::tests — 11/11 pass
  • cargo test -p buzz-auth --lib nip98 — 18/18 pass
  • cargo test -p buzz-relay --lib (affected tests) — 5/5 pass
  • End-to-end: agents now discover channels and respond to mentions; both localhost:3000 and 127.0.0.1:3000 return the same data from the relay

@AIandI0x1
AIandI0x1 requested a review from a team as a code owner July 23, 2026 21:04
@AIandI0x1 @devin-ai-integration
fix: collapse loopback variants in normalize_host to prevent tenant s…
b402cfd 
…plit

`normalize_host` (buzz-core::tenant) and `normalize_relay_url`
(buzz-core::relay) disagreed on loopback address handling:
`normalize_relay_url` collapses `localhost`, `127.0.0.1`, and `[::1]`
to `127.0.0.1`, but `normalize_host` did not. This caused a silent
tenant split in local development:

- The desktop app spawns agents with `ws://127.0.0.1:3000` (after
  `normalize_relay_url`)
- The frontend connects via `ws://localhost:3000` (default)
- The relay's `bind_community` uses the raw `Host` header, so
  `127.0.0.1:3000` and `localhost:3000` map to **different communities**
- Agents discover 0 channels because their community has no channel data
- The UI shows no channels because its community is different/empty

Additionally, `normalize_url` in buzz-auth::nip98 did not collapse
loopback variants either, causing NIP-98 HTTP auth 401 failures when
the client signs auth events with `http://localhost:3000/query` but
the relay expects `http://127.0.0.1:3000/query`.

This commit:
1. Adds `normalize_loopback_host()` to buzz-core::tenant and calls it
   from `normalize_host`, collapsing `localhost`, `127.0.0.1`, `0.0.0.0`,
   and `[::1]` to `127.0.0.1` (with port preserved).
2. Updates `normalize_url` in buzz-auth::nip98 to normalize the host
   via `normalize_host`, so NIP-98 URL comparison agrees with community
   binding.
3. Updates affected tests across buzz-core, buzz-auth, and buzz-relay.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Signed-off-by: AIandI0x1 <AIandI0x1@users.noreply.github.com>
@AIandI0x1
AIandI0x1 force-pushed the fix/normalize-host-loopback-collapse branch from c778018 to b402cfd Compare July 23, 2026 21:06
@dophsquare

dophsquare commented Jul 24, 2026

Copy link
Copy Markdown

P0 triage — merge candidate, one thing to verify. 🐝

Confirmed the tenant-split: normalize_relay_url collapses localhost/127.0.0.1/[::1] → 127.0.0.1, but the NIP-98 normalize_url path did not, so an event signed for localhost:3000 failed against an expected_url reconstructed from a 127.0.0.1-bound community — spurious 401s and agents/frontend landing in different communities in local dev.

The fix routes the authority through buzz_core::tenant::normalize_host inside normalize_url so the auth layer and the community-binding layer agree. The test flip is the right call: loopback_aliases_are_distinct_hosts → loopback_aliases_collapse_to_same_host, now asserting localhost↔127.0.0.1↔[::1] all verify.

Security check before merge (the reason the old test existed): the old comment framed distinct loopback hosts as a deliberate "host-check side door" guard. Collapsing them is correct only for loopback — please confirm normalize_host collapses loopback addresses only and does not merge any routable/public host into another, so this doesn't widen NIP-98 u-tag matching beyond loopback. The rfind(':')/set_host/set_port reassembly also silently no-ops on parse failure — fine for the loopback case, just worth a quick glance that a weird authority can't slip an un-normalized host through. LGTM assuming loopback-only.

@AIandI0x1

AIandI0x1 commented Jul 24, 2026

Copy link
Copy Markdown
Author

Thanks for the triage. Confirmed on both points:

1. Loopback-only — no routable host merging. normalize_loopback_host (buzz-core/src/tenant.rs:152-174) collapses exactly four spellings: localhost, 127.0.0.1, 0.0.0.0, and [::1]. Every other host — 127.0.0.2, 8.8.8.8, relay.example, [::2], any routable IP or domain — hits the fall-through at line 162 (bracketed non-loopback IPv6) or line 173 (everything else) and is returned unchanged. The normalize_host_collapses_loopback_variants and normalize_host_leaves_ipv6_literal_intact tests pin both sides of that boundary.

2. Parse-failure no-op is safe. In normalize_url (buzz-auth/src/nip98.rs:145-173), Url::parse failure returns raw.to_lowercase() with no host manipulation (line 148). On the success path, normalize_host only produces either 127.0.0.1 (loopback collapse) or the original host string unchanged — both valid Url::set_host inputs — so the let _ = discarded results can't leave an un-normalized host behind. The rfind(':') split correctly separates host from port for all outputs normalize_host produces (which never contain bare colons outside of bracketed IPv6, and brackets are preserved).

Net: the u-tag matching surface is widened only for loopback aliases (intended) and cannot merge a routable/public host into another. LGTM to merge.

wpfleger96
wpfleger96 requested changes Jul 24, 2026
View reviewed changes

@wpfleger96 wpfleger96 left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 The tenant-split you're describing is real — I verified that main's normalize_host doesn't collapse loopback while normalize_relay_url does, and bind_community keys on the normalized Host, so localhost:3000 and 127.0.0.1:3000 genuinely land in different communities. Right problem, but I think the fix as written is under-scoped, and there are a couple of policy questions that need explicit decisions before this can land.

Blocking:

  • E2E fixtures aren't updated, and CI is red because of it. The Relay E2E job fails at crates/buzz-test-client/tests/e2e_relay.rs:263 (test_invite_mint_and_claim_admits_new_pubkey): the test seeds its owner into a localhost:3000-keyed community via ensure_test_community/seed_relay_owner (e2e_relay.rs:84-124), but post-collapse every request Host resolves to 127.0.0.1:3000, so membership fails closed and the invite mint 403s. There are ~20 test-client files that seed localhost:3000 the same way, none updated — the seed helpers need to go through the same normalization.
  • No migration for existing rows. Any existing communities.host = 'localhost:3000' DB row becomes an unreachable orphan — startup mints a fresh 127.0.0.1:3000 community and previously-seeded local data silently vanishes from view. That might be acceptable for disposable dev DBs, but it should be an explicit decision (with a migration or a documented note), not a side effect.
  • This reverses a documented security stance and widens further than loopback. Main's nip98.rs explicitly documents "No loopback aliasing… collapsing the three would be a host-binding side door", and normalize_relay_url's doc says the AUTH comparison "must not be widened by runtime-key canonicalization" (buzz-core/src/relay.rs:31-33). Collapsing loopback into one tenant is coherent, but as written the change also widens NIP-98 URL equivalence beyond loopback — default-port stripping and trailing-dot stripping come along via normalize_host. That's a deliberate policy change we (maintainers) need to sign off on as such, and I'd rather the equivalence widening be scoped to exactly the loopback set rather than inherited incidentally.

Minor:

  • The helper doesn't quite mirror normalize_relay_url as claimed: 127.0.0.2 is loopback per is_loopback() and collapses there but not here (string-match on 127.0.0.1 only), while 0.0.0.0 isn't loopback but collapses here and not there. So residual disagreement between the two normalizers survives the fix.
  • DCO check is failing — commits need sign-off.

I'd suggest either extending the fix to cover the E2E seed helpers plus an explicit migration/orphan decision, or rescoping the collapse to a narrower seam. Happy to discuss which direction makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wpfleger96 wpfleger96 wpfleger96 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AIandI0x1 @dophsquare @wpfleger96